-
Notifications
You must be signed in to change notification settings - Fork 10
Attached (External) Subnets #911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
374d344 to
a832697
Compare
a832697 to
5dc70c8
Compare
This PR reworks certain aspects of how NAT and gateway layers are constructed to enable attached (external) subnets to function. The primary aim here is that traffic to/from these subnets (owned by the host) do not undergo NAT, and bypass spoof detection as transit IPs would. A few wider changes have been necessary to ensure that these can be attached/detached without breaking any existing transit IPs, and to ensure that traffic originated from an external subnet cannot be directed towards a private VPC recipient.
5dc70c8 to
2e7cf69
Compare
| use opte_api::Protocol; | ||
|
|
||
| pub trait Ip: Into<super::IpAddr> { | ||
| pub trait Ip: Into<super::IpAddr> + Send + Sync { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Followup from #883 -- this moves the Send + Sync bounds up so that we don't need to explicitly specify them in as many spots.
| pub fn attach_subnet( | ||
| port: &Port<VpcNetwork>, | ||
| inet_gw_map: Option<&InternetGatewayMap>, | ||
| vpc_mappings: &Arc<VpcMappings>, | ||
| req: AttachSubnetReq, | ||
| ) -> Result<(), OpteError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a bit of uncertainty as to whether this and detach_subnet should live here, gateway, or some other module since the feature crosses multiple layers.
bnaecker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not looked through the entire PR yet, but this seems solid. Nice interface for attaching subnets, and some helpful cleanup along the way. Thanks!
This PR reworks certain aspects of how NAT and gateway layers are constructed to enable attached (external) subnets to function. The primary aim here is that traffic to/from these subnets (owned by the host) do not undergo NAT, and bypass spoof detection as transit IPs would. A few wider changes have been necessary to ensure that these can be attached/detached without breaking any existing transit IPs, and to ensure that traffic originated from an external subnet cannot be directed towards a private VPC recipient.
VpcCfgrather than working directly on the port's ruleset. This also gives us the opportunity to set these on port create rather than using a followup ioctl.DhcpCfginVpcCfgnow.VpcCfginXdeDevand the port itself seemed... misleading, at best. When XDE needs that, we now reach throughport.network().StatefulActions should never have had mutable access to a packet'sActionMeta(the string-to-string map), because they may never be rerun after an LFT is created on subsequent slowpath walks. It is now up to the generatedActionDescs to update metadata -- this trait's role is similar toStaticAction, which is allowed mutable access to this metadata.ActionMetaergonomics to make this easier to express and implement.As far as the control plane is concerned, it should need to only:
Instance(<name>)target (in turn resolved to an IP target when reifying rules for the port, as happens with instance targets today).Answers the functional dataplane requirements of #890. Closes #703.